Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Right panel chat style changes for read receipts and optimizations for smaller widths #7297

Merged
merged 7 commits into from
Dec 13, 2021

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Dec 6, 2021

Fixes: element-hq/element-web#19905
This is a first draft for the new styling.
It tires to accommodate the read receipts and use the space more efficiently when there is little space available like in the right panel chat.
It needs feedback from design!
The two options differ in how aggressive they use horizontal space:

  • Option 1 really similar to the normal design just a little bit tighter
  • Option 2 uses all the possible area on the left.

Option 1

option1

Option 2

option2

Before:


This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://61b75240c441b909d346564a--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@toger5 toger5 requested a review from a team as a code owner December 6, 2021 15:41
@jryans jryans requested a review from gaelledel December 6, 2021 16:09
@jryans jryans changed the title Right panel chat style changes for read receites and optimizations for smaller width's Right panel chat style changes for read receipts and optimizations for smaller widths Dec 6, 2021
@jryans jryans removed the request for review from a team December 8, 2021 10:25
@jryans
Copy link
Collaborator

jryans commented Dec 8, 2021

I think we should start with a Design review of this one first, as there are several proposed options. After that's settled, we can continue into the code review.

Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toger5 @jryans

For the full width of the pannel:
Surface out in this order: Emoji , Overflow
Inside the Overflow: Stickers, attachments, Voice message, Polls, Location sharing
https://www.figma.com/file/DFBvBKfg2sZw6YrbqOYWK6/Composer?node-id=317%3A12520

If required when reducing the view port size, hide the emoji button inside the overflow with emoji placed at the top.

@toger5
Copy link
Contributor Author

toger5 commented Dec 8, 2021

@gaelledel This PR is not intended to modify the composer. Even though that is also interesting. I agree that the emoji picker should be prioritized to the attachment button.
The intention was to only get read recites added, so that the right panel chat is closer to being feature complete.
Feel free to open another issue about the composer. I think it would be best to have this issue only focused on the timeline.

@gaelledel
Copy link

@gaelledel This PR is not intended to modify the composer. Even though that is also interesting. I agree that the emoji picker should be prioritized to the attachment button. The intention was to only get read recites added, so that the right panel chat is closer to being feature complete. Feel free to open another issue about the composer. I think it would be best to have this issue only focused on the timeline.

Right gotcha and apologies. In this case, the Proposal you have there works. Will create an issue for the composer (fyi I meant to sync with Jano about exposing emoji for the threads panel too but he's not been available)

@toger5
Copy link
Contributor Author

toger5 commented Dec 8, 2021

@gaelledel what approach do you prefer:

  • you and jano choose one of the two options and we mere this rather quickly? (with still having the option to open an issue and improve on it when we have a Figma design) Which of the two option should we go with than?
  • we keep this as a first building block and wait for a proper Figma design which I than (hopefully) can execute based on this.

I really like/need/use the read recites so I am biased to have them in the right panel rather quickly.

@gaelledel
Copy link

@gaelledel what approach do you prefer:

  • you and jano choose one of the two options and we mere this rather quickly? (with still having the option to open an issue and improve on it when we have a Figma design) Which of the two option should we go with than?
  • we keep this as a first building block and wait for a proper Figma design which I than (hopefully) can execute based on this.

I really like/need/use the read recites so I am biased to have them in the right panel rather quickly.

Yes so you can merge this work on read receipts - For the emoji and composer we will see later

Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this and we can figure out the composer stuff later

@toger5
Copy link
Contributor Author

toger5 commented Dec 9, 2021

@gaelledel : which of the two options from the PR description?
@jryans : I think I should than also add a listener for the read recite setting? (I will request a code review again since this has green light from design now)

@toger5
Copy link
Contributor Author

toger5 commented Dec 13, 2021

Settings updates are tested now. I prefer Option 1. So I am going to merge this this evening if there are not further objections (I'll keep the comment for the other option in case we want to change that later. If this is bad practice I will remove them before merging)

@jryans
Copy link
Collaborator

jryans commented Dec 13, 2021

So I am going to merge this this evening if there are not further objections

Well, just to clarify process-wise, no one has approved the code yet, so that's needed first. 😉 Anyway, I will review it now.

@jryans jryans self-requested a review December 13, 2021 13:06
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost ready, just a small question to resolve. 😄

res/css/views/right_panel/_TimelineCard.scss Outdated Show resolved Hide resolved
src/components/structures/RightPanel.tsx Show resolved Hide resolved
@toger5
Copy link
Contributor Author

toger5 commented Dec 13, 2021

Well, just to clarify process-wise, no one has approved the code yet, so that's needed first. wink Anyway, I will review it now.

sorry you are right. There was no code review yet. Thanks taking the time to review now!

@toger5 toger5 requested a review from jryans December 13, 2021 13:59
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me! 😄

@toger5 toger5 merged commit f938bfa into develop Dec 13, 2021
@toger5 toger5 deleted the toger5/timelineCard_styling branch December 13, 2021 16:46
@@ -33,4 +33,55 @@ limitations under the License.
right: 0;
Copy link
Contributor

@MadLittleMods MadLittleMods Dec 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @toger5's demo of maximized widgets where it showed the right-panel chat style, I was surprised that it looked a lot more native and refined compared to the thread view. This probably comes down to the differences from the normal timeline view because we're changing some the pieces around like the timestamp position in the thread view. I understand that the right panel chat style and the thread view are different.

Right panel chat style that feels nice to me:

The unpolished feeling may apply to this PR iteration of the right-panel chat where things are getting condensed like the thread view but it's hard to judge because I haven't actually used it in the app like threads and it's nice to be able to judge the balance against the whole app and timeline view.

As @toger5 notes, some adaptations probably need to be made for the smaller layout,

We definitely have to make changes to the style. Just to get read-receipts not glitching/overlapping with the messages. The question is, if the style changes are too aggressive. I see both sides. It looking more like the main timeline (more native) when changing as a little as possible and adapting it to the right panel so the small area is used as efficient as possible.

-- @toger5, https://matrix.to/#/!tCaHWuHHvOHjWyqapP:matrix.org/$UibGBNcTpdbe-0zdRn1Ktp8_yHWYh9wL6HlfxJ3t-tc?via=matrix.org&via=element.io&via=uhoreg.ca


As a tiny point of reference, Slack and Discord are able to maintain the same layout between the main message view and threads but they have a different timestamp position and less UI (don't need to contend with read-receipts).

Slack image referencesSlack timestamp position for a burst of messages:

Slack thread view (bursts are always separated):

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize space in right panel chat and threads
4 participants